-
Notifications
You must be signed in to change notification settings - Fork 66
feat(23570): Add controller for workspace backup #1530
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Allda The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
42dd45c to
dffd7e6
Compare
|
@Allda : Really appreciate you taking the time to contribute this in such a short time. 🎉 Could you please also fill out the “Is it tested? How?” section in the PR template? It’ll help reviewers and future contributors verify the change more easily. Thanks again for your effort! 🙌 |
|
I tested this PR and it seems to work.
config:
workspace:
backupCronJob:
enable: true
schedule: "*/3 * * * *"
|
0bc74b1 to
8427ba5
Compare
|
/retest |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1530 +/- ##
==========================================
+ Coverage 34.09% 35.30% +1.21%
==========================================
Files 160 161 +1
Lines 13348 13802 +454
==========================================
+ Hits 4551 4873 +322
- Misses 8487 8599 +112
- Partials 310 330 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ibuziuk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A backup job use a PVC name from a default value or from the config if user configured custom name. Signed-off-by: Ales Raszka <[email protected]>
The backup job can now push to registries which requires auth token. The token is provided as a secret in operator namespace and added to the operator config. Signed-off-by: Ales Raszka <[email protected]>
Signed-off-by: Ales Raszka <[email protected]>
A backup job now determines the name of pvc based on used storage type. It distinguish between different storage types (common and per-workspace) and mount the volume dynamically. Signed-off-by: Ales Raszka <[email protected]>
It turns out the capabilities from the prototype are not needed. Signed-off-by: Ales Raszka <[email protected]>
A new SA is created for the backup jobs to limit the permission to just what is necessary. Signed-off-by: Ales Raszka <[email protected]>
Signed-off-by: Ales Raszka <[email protected]>
- Make registry field required - Replace custom bool comparison with library - Minor tweeks Signed-off-by: Ales Raszka <[email protected]>
Use single logger across the controller and only add context if needed. Signed-off-by: Ales Raszka <[email protected]>
|
@Allda maybe I'm missing something but I am getting this error: This is my DWOC: Any ideas? |
Could you please share more details about the created Job? Are there any events or logs? I tried it today with a similar config (only difference is registry location) and the push Job passed. |
|
Here is the job's yaml: job-devworkspace-backup-29mb2.yaml There weren't any pod logs (since the pod never started) but only a lot of the Screen.Recording.2025-11-18.at.3.59.33.PM.movOutput of |
A registry configuration is now stored under a separated nested struct. Signed-off-by: Ales Raszka <[email protected]>
A SA is created for every backup workspace to avoid ownership conflict. Signed-off-by: Ales Raszka <[email protected]>
A switching a based image to podman allowed us to run a backup job as a regular user 1000 without any privileged escalation. Signed-off-by: Ales Raszka <[email protected]>
Signed-off-by: Ales Raszka <[email protected]>
Signed-off-by: Ales Raszka <[email protected]>
|
@Allda : Edit: I was able to resolve this issue by granting additional permissions to ServiceAccount , however I'm not sure whether this is required step or some issue: After updating ServiceAccount permissions, I'm able to see backup job getting executed and creating images on the configured registry:
Question: Will the backup image will be platform dependent? I see only I'm also facing the same issue as David. I was trying to test your changes on CRC. Environment:OS: Linux CRC Version: Steps to Reproduce:
config:
workspace:
backupCronJob:
enable: true
registry:
authSecret: dockerhub-push-secret
path: docker.io/rohankanojia
schedule: '* * * * *'
# run From DWO root dir
oc create -f samples/code-latest.yaml
oc patch devworkspace code-latest \
--type=merge \
-p '{"spec": {"started": false}}'
Upon checking details I see this error: Perhaps pod creation is getting forbidden by OpenShift SecurityContextConstraints due to this: SecurityContext: &corev1.SecurityContext{
RunAsUser: ptr.To[int64](1000),
}, |
|
@rohanKanojia @dkwon17 I managed to find an alternative that doesn't violate any OCP-specific security constraints. The I tested it locally on Kind and remotely on OCP 4.20 cluster. |
Signed-off-by: Ales Raszka <[email protected]>
|
@Allda : Thank you! I can confirm that this approach works without any explicit configuration. I can see the backup image being pushed to the configured registry. However, I see image has a different format I tested on CRC with OpenShift 4.20.1 |
|
|
||
| // ExtraArgs are additional arguments passed to the oras CLI | ||
| // +kubebuilder:validation:Optional | ||
| ExtraArgs string `json:"extraArgs,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the intended use case for passing arbitrary oras CLI flags? Should we allow users to inject raw CLI arguments, given that this might expose the underlying backup implementation?
| Path string `json:"path,omitempty"` | ||
| // AuthSecret is the name of a Kubernetes secret of | ||
| // type kubernetes.io/dockerconfigjson | ||
| // +kubebuilder:validation:Optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it would help clarify that the AuthSecret must reside in the same namespace as the DevWorkspaceOperatorConfig?

A new backup controller orchestrates a backup process for workspace PVC. A new configuration option is added to DevWorkspaceOperatorConfig that enables running regular cronjob that is responsible for backup mechanism. The job executes following steps:
The last step is currently not fully implemented as it requires running a buildah inside the container and it will be delivered as a separate feature.
Issue: eclipse-che/che#23570
What does this PR do?
What issues does this PR fix or reference?
Is it tested? How?
The feature has been tested locally and using integration tests. Following configuration should be added to the config to enable this feature:
After a config is added, stop any workspace and wait till a backup job is created.
The job creates a backup and push image to registry
PR Checklist
/test v8-devworkspace-operator-e2e, v8-che-happy-pathto trigger)v8-devworkspace-operator-e2e: DevWorkspace e2e testv8-che-happy-path: Happy path for verification integration with Che